Add processed_mentions tracking to prevent duplicate replies and improve fallback message - #110
Add processed_mentions tracking to prevent duplicate replies and improve fallback message#110groupthinking wants to merge 1 commit into
Conversation
📝 WalkthroughSummary
Walkthrough
ChangesMention processing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant X_API
participant listener
participant Grok
participant processed_mentions_file
X_API->>listener: provide mentions
listener->>listener: skip processed IDs
listener->>Grok: request reply
Grok-->>listener: reply or failure
listener->>X_API: create reply tweet
listener->>processed_mentions_file: persist mention ID
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds persistent tracking of processed X mention IDs in the Python listener to prevent duplicate replies across polling cycles/restarts, and updates the fallback reply message when Grok generation fails.
Changes:
- Persist processed mention IDs to a local file and skip already-processed mentions.
- Update the Grok error fallback reply text.
- Change the listener’s default Timeline API URL (currently to port 8000).
Comments suppressed due to low confidence (1)
listener.py:100
- The default TIMELINE_API_URL port was changed to 8000 here, but the rest of the repo (env.example, docker-compose, mcp_dispatcher.py) still defaults the timeline server to 8080. If TIMELINE_API_URL isn’t set, agent registration will POST to the wrong local endpoint.
timeline_url = os.getenv("TIMELINE_API_URL", "http://127.0.0.1:8000")
|
|
||
| def push_timeline_card(title: str, body: str, metadata: dict) -> None: | ||
| timeline_url = os.getenv("TIMELINE_API_URL", "http://127.0.0.1:8080") | ||
| timeline_url = os.getenv("TIMELINE_API_URL", "http://127.0.0.1:8000") |
| mention_id_str = str(mention.id) | ||
| if mention_id_str in processed_mentions: | ||
| print(f"Skipping already processed mention {mention.id}", flush=True) | ||
| continue | ||
|
|
| try: | ||
| client.create_tweet( | ||
| text=grok_reply[:280], | ||
| in_reply_to_tweet_id=mention.id, | ||
| ) | ||
| processed_mentions.add(mention_id_str) | ||
| save_processed_mention(mention_id_str) | ||
| except Exception as exc: | ||
| print(f"Error replying to mention {mention.id}: {exc}", flush=True) |
| def load_processed_mentions() -> set[str]: | ||
| if not PROCESSED_MENTIONS_PATH.exists(): | ||
| return set() | ||
| with PROCESSED_MENTIONS_PATH.open("r", encoding="utf-8") as f: | ||
| return {line.strip() for line in f if line.strip()} |
| POLL_SECONDS = int(os.getenv("POLL_INTERVAL_SECONDS", "60")) | ||
| PAYMENT_REQUIRED_BACKOFF_SECONDS = int(os.getenv("X_PAYMENT_REQUIRED_BACKOFF_SECONDS", "900")) | ||
|
|
||
| PROCESSED_MENTIONS_PATH = Path(os.getenv("XMCP_PROCESSED_MENTIONS_PATH", "~/.xmcp/processed_mentions.txt")).expanduser() |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@listener.py`:
- Around line 45-48: Replace the unbounded append-only storage in
save_processed_mention with a bounded durable store, or add safe
compaction/retention tied to the listener’s replay window. Ensure compaction
preserves every mention ID still needed to prevent replay while removing older
entries, and keep startup loading bounded accordingly.
- Around line 171-174: Update the get_grok_reply flow in the mention handler so
failure-like return values such as “Missing XAI_API_KEY.” and “Thinking...” are
detected before create_tweet(). Route them through the existing apology fallback
and ensure they are not published or recorded as successful replies; preserve
the current exception handling behavior for raised errors.
- Around line 38-42: Update load_processed_mentions() to handle filesystem read
errors without silently returning an empty set: retry transient failures or
propagate an explicit health failure. Adjust main() so failures from
load_processed_mentions() are handled within the daemon polling lifecycle,
preventing the listener thread from terminating while preserving the fail-closed
behavior that avoids duplicate replies.
- Around line 176-182: The mention handler around create_tweet and
save_processed_mention must distinguish successful tweet delivery from durable
state persistence. Do not let save_processed_mention failures enter the “Error
replying” path or treat the tweet as unsent; make persistence failures retry or
fail closed, and only update processed_mentions consistently with a successful
durable commit to prevent replay after restart.
- Line 86: Align the TIMELINE_API_URL fallback consistently across listener.py,
env.example, and agents/base.py, using the same endpoint default everywhere
(preferably port 8080 to match the existing configuration). Update the listener
registration and timeline-card request paths and the documented example
together, or make the variable mandatory in all locations.
- Around line 156-159: Update the mention-processing flow around the
processed_mentions check so the persisted start_time/cursor advances
monotonically to the newest processed mention, rather than being overwritten for
each newest-first item. Track the maximum tweet ID or timestamp across handled
mentions and persist that value only after processing the batch, while
preserving the skip behavior for already processed mentions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e25f4c6-1faa-49f0-b6cb-fd85c8c5c777
📒 Files selected for processing (1)
listener.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
groupthinking/uvai-skills(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: copilot-pull-request-reviewer
🧰 Additional context used
🔍 Remote MCP GitHub Copilot
Additional PR context
- PR
#110is open, has 1 commit, 1 changed file (listener.py), with +26/-4 lines; mergeable state isdirty. An auto-generated CodeRabbit comment says review is still in progress. main’slistener.pyalready persistslast_seenand processes mentions oldest-first; this PR adds a separate persistedprocessed_mentionsset viaXMCP_PROCESSED_MENTIONS_PATH, loads it at startup, skips already-processed mention IDs, and saves IDs after a successful reply.- The Grok-failure fallback text changes from
Processing your tag... (error generating full response)toSorry, I'm having trouble processing that. Try again or DM me. - The timeline API default in
listener.pychanges tohttp://127.0.0.1:8000, butenv.exampleandagents/base.pystill defaultTIMELINE_API_URLtohttp://127.0.0.1:8080;env.examplealso setsMCP_PORT=8000andMCP_SERVER_URL=http://127.0.0.1:8000/mcp.
🔇 Additional comments (1)
listener.py (1)
19-19: LGTM!Also applies to: 129-130, 140-141
| def load_processed_mentions() -> set[str]: | ||
| if not PROCESSED_MENTIONS_PATH.exists(): | ||
| return set() | ||
| with PROCESSED_MENTIONS_PATH.open("r", encoding="utf-8") as f: | ||
| return {line.strip() for line in f if line.strip()} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not let a broken state file kill the listener.
load_processed_mentions() lets filesystem errors escape, and main() calls it outside the polling error handler. Because main.py:49-58 runs this in a daemon thread, an unreadable or temporarily unavailable file terminates mention processing. Retry or fail closed with an explicit health failure; never silently use an empty set, since that re-enables duplicate replies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 38 - 42, Update load_processed_mentions() to handle
filesystem read errors without silently returning an empty set: retry transient
failures or propagate an explicit health failure. Adjust main() so failures from
load_processed_mentions() are handled within the daemon polling lifecycle,
preventing the listener thread from terminating while preserving the fail-closed
behavior that avoids duplicate replies.
|
|
||
| def push_timeline_card(title: str, body: str, metadata: dict) -> None: | ||
| timeline_url = os.getenv("TIMELINE_API_URL", "http://127.0.0.1:8080") | ||
| timeline_url = os.getenv("TIMELINE_API_URL", "http://127.0.0.1:8000") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align the timeline endpoint default everywhere.
listener.py now defaults to port 8000, while env.example and agents/base.py still default 8080. With no explicit TIMELINE_API_URL, registration and timeline-card requests hit the wrong service. Update all defaults together or require the variable explicitly.
Also applies to: 100-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` at line 86, Align the TIMELINE_API_URL fallback consistently
across listener.py, env.example, and agents/base.py, using the same endpoint
default everywhere (preferably port 8080 to match the existing configuration).
Update the listener registration and timeline-card request paths and the
documented example together, or make the variable mandatory in all locations.
| grok_reply = get_grok_reply(prompt) | ||
| except Exception as exc: | ||
| print(f"Error getting Grok reply for mention {mention.id}: {exc}", flush=True) | ||
| grok_reply = "Processing your tag... (error generating full response)" | ||
| grok_reply = "Sorry, I'm having trouble processing that. Try again or DM me." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Route all failed Grok results through the apology fallback.
get_grok_reply() returns "Missing XAI_API_KEY." and "Thinking..." for failure-like paths instead of raising. This handler therefore publishes and records those placeholders as successful replies. Return a typed success/error result or raise for these cases before calling create_tweet().
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 172-172: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 171 - 174, Update the get_grok_reply flow in the
mention handler so failure-like return values such as “Missing XAI_API_KEY.” and
“Thinking...” are detected before create_tweet(). Route them through the
existing apology fallback and ensure they are not published or recorded as
successful replies; preserve the current exception handling behavior for raised
errors.
| try: | ||
| client.create_tweet( | ||
| text=grok_reply[:280], | ||
| in_reply_to_tweet_id=mention.id, | ||
| ) | ||
| processed_mentions.add(mention_id_str) | ||
| save_processed_mention(mention_id_str) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not treat state-persistence failure as tweet failure.
If create_tweet() succeeds but save_processed_mention() fails, this block logs “Error replying” after already sending the reply. The ID is also added to memory before the disk write, so only the current process suppresses a duplicate; after restart, the reply is sent again. Separate the external write from the durable state commit and retry/fail closed on persistence errors instead of blindly replaying a non-idempotent tweet.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 176 - 182, The mention handler around create_tweet
and save_processed_mention must distinguish successful tweet delivery from
durable state persistence. Do not let save_processed_mention failures enter the
“Error replying” path or treat the tweet as unsent; make persistence failures
retry or fail closed, and only update processed_mentions consistently with a
successful durable commit to prevent replay after restart.
|
Do not resolve the conflicts on this PR. Re-cut the branch from current I audited all five open PRs (#106–#110) against the GitHub compare API. Every one of them is a long-stale branch, not a genuine content conflict:
All five report Why the diffs look absurd#109 is titled "fix: replace invalid CODEOWNERS entries with @groupthinking" but reports +30,880 / −216 across 38 files, with essentially every file showing That is not what the PR intends to change. The branch was cut on 2026-01-27 and opened as a PR on 2026-07-24 — a six-month gap during which The conflicts are therefore real, but they are drift, not disagreement. Resolving them by hand means manually reconciling six months of divergence across 38 files to land what should be a handful of line edits — and every manual resolution is an opportunity to silently revert work that landed on ResolutionFor each PR, the actual intent is small and is captured in only 1–12 commits. Re-apply that intent on top of current git fetch origin
git checkout -b <name>-rebased origin/main
git cherry-pick <the 1-12 real commits> # or simply re-make the edit by handThen open a replacement PR and close the stale one. For #109 specifically, the intended change is a CODEOWNERS edit — that is a few lines, and re-making it by hand against current Recommended disposition:
Root cause to fix going forward: these five PRs were all opened within a 112-second window (22:41:30 → 22:43:22 on 2026-07-24) from branches that were months old. Whatever automation opened them did not rebase first. Adding a staleness check — refuse to open a PR whose branch is more than N commits behind its base — would prevent this class of PR entirely. |
…lback reply Re-applies the intent of the original fix-spam commit on top of current main, per the review decision to re-cut rather than hand-resolve six months of branch drift. - listener.py: persist replied-to mention IDs (XMCP_PROCESSED_MENTIONS_PATH) and skip them on re-fetch — the inclusive start_time watermark re-returns the boundary mention on every restart. The watermark still advances on skips, the ledger is compacted to XMCP_MAX_PROCESSED_MENTIONS on load, an unreadable ledger degrades to an empty set instead of killing the listener thread, and persistence failures are logged distinctly from reply failures. - agents/team/general.py: replace the "Thinking..." placeholder published when Grok returns nothing with an apology fallback. - env.example: document the new state variables. The original commit's TIMELINE_API_URL port change (8080 -> 8000) is dropped per review feedback — the rest of the repo defaults to 8080. Co-authored-by: linear-code[bot] <222613912+linear-code[bot]@users.noreply.github.com>
🔍 PR Validation❌ PR description is required (minimum 20 characters) |
🔍 PR Validation |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
listener.py:275
processed_mentionsis only compacted on startup, but it grows for the entire lifetime of the listener (processed_mentions.add(...)on every successful mention). In a long-running process this can grow without bound (and contradicts the intent in the comment that the ledger only needs to cover a replay window). The TypeScript agent (src/services/agent.ts:101-107) prunes its processed set to avoid this.
Consider bounding the in-memory set here (and optionally add periodic on-disk compaction if the ledger file is expected to grow large between restarts).
processed_mentions.add(mention_id)
try:
save_processed_mention(mention_id)
except OSError as exc:
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
listener.py (1)
76-79: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winLedger only compacts at startup; a long-running process still grows it unbounded.
save_processed_mention()unconditionally appends. Compaction againstMAX_PROCESSED_MENTIONSonly runs insideload_processed_mentions(), called once inmain(). Between restarts, this daemon thread's ledger file grows without limit — the documented cap only takes effect the next time the process restarts. This is the same unbounded-growth concern already raised, now only half-fixed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@listener.py` around lines 76 - 79, Update save_processed_mention so the processed-mentions ledger is compacted during runtime, not only by load_processed_mentions at startup. After appending the new mention ID, enforce MAX_PROCESSED_MENTIONS by retaining only the newest allowed entries, while preserving the existing file format and append behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@listener.py`:
- Around line 45-66: Update load_processed_mentions so an OSError while reading
PROCESSED_MENTIONS_PATH does not return an empty processed-mentions set; retry
the ledger read or fail closed by preserving duplicate suppression. Adjust the
docstring and warning to reflect the chosen behavior, while keeping normal
loading and compaction unchanged.
- Around line 265-275: Update the exception path around save_processed_mention
in the mention-processing flow so an OSError does not allow start_time to
advance past an unpersisted mention; stop or otherwise retry processing before
advancing the checkpoint, while preserving the existing warning context and
normal success behavior.
- Around line 67-68: Update the truncation logic near MAX_PROCESSED_MENTIONS to
handle zero and negative limits explicitly: a non-positive cap must produce an
empty lines list, while positive caps retain only the last
MAX_PROCESSED_MENTIONS entries when the list exceeds the limit.
- Around line 15-23: Ensure load_env() runs before PROCESSED_MENTIONS_PATH and
MAX_PROCESSED_MENTIONS are evaluated, or resolve both settings lazily after
environment loading in main(). Preserve the existing environment variable names
and defaults so .env values control the ledger path and cap.
---
Duplicate comments:
In `@listener.py`:
- Around line 76-79: Update save_processed_mention so the processed-mentions
ledger is compacted during runtime, not only by load_processed_mentions at
startup. After appending the new mention ID, enforce MAX_PROCESSED_MENTIONS by
retaining only the newest allowed entries, while preserving the existing file
format and append behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dff295e5-dc01-408d-a561-97665eecf8f5
📒 Files selected for processing (3)
agents/team/general.pyenv.examplelistener.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: copilot-pull-request-reviewer
🧰 Additional context used
🪛 ast-grep (0.45.0)
listener.py
[warning] 105-105: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(f"{timeline_url}/v1/timeline/items", json=payload, timeout=10)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
🪛 Ruff (0.16.0)
agents/team/general.py
[warning] 8-8: typing.Dict is deprecated, use dict instead
(UP035)
[warning] 22-22: Missing return type annotation for special method __init__
Add return type annotation: None
(ANN204)
listener.py
[warning] 134-134: Do not catch blind exception: Exception
(BLE001)
[warning] 157-157: Consider moving this statement to an else block
(TRY300)
[warning] 158-158: Do not catch blind exception: Exception
(BLE001)
[warning] 164-164: Do not catch blind exception: Exception
(BLE001)
[warning] 176-176: Do not catch blind exception: Exception
(BLE001)
[warning] 195-195: Do not catch blind exception: Exception
(BLE001)
🔍 Remote MCP GitHub Copilot
Relevant review context
- The PR’s current diff only changes
listener.py,agents/team/general.py, andenv.example; no tests cover the new ledger behavior. PROCESSED_MENTIONS_PATHandMAX_PROCESSED_MENTIONSare evaluated at module import time, whileload_env()runs later insidemain(). Therefore values supplied only through.envmay not configure the new ledger.MAX_PROCESSED_MENTIONS=0is not safely bounded:lines[-0:]retains all entries, so the documented maximum can be bypassed.- The ledger is appended after processing, while
save_last_seen()follows afterward. A crash between those writes can leave the mention recorded as processed but the watermark unchanged; the next poll will skip it and then advance the watermark. This is consistent with the intended duplicate suppression but should be covered by tests. - The PR branch is reported as 53 commits behind
mainand divergent; the author explicitly recommends re-cutting it from currentmainrather than resolving conflicts manually. - The automated review reported no Copilot approval, no AI unit-test label/tests for this PR, and an inconclusive description check.
🔇 Additional comments (5)
listener.py (2)
95-109: LGTM!
112-160: LGTM!env.example (1)
45-47: LGTM!agents/team/general.py (2)
34-55: LGTM!
57-70: LGTM!
| LAST_SEEN_PATH = Path(os.getenv("XMCP_LAST_SEEN_PATH", "~/.xmcp/last_seen.txt")).expanduser() | ||
| PROCESSED_MENTIONS_PATH = Path( | ||
| os.getenv("XMCP_PROCESSED_MENTIONS_PATH", "~/.xmcp/processed_mentions.txt") | ||
| ).expanduser() | ||
| # The last-seen watermark has second granularity and start_time is inclusive, | ||
| # so the newest processed mention is re-fetched on every restart. The | ||
| # processed-mentions ledger exists to suppress that duplicate reply; it only | ||
| # needs to cover the replay window, not all history. | ||
| MAX_PROCESSED_MENTIONS = int(os.getenv("XMCP_MAX_PROCESSED_MENTIONS", "10000")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -a 'listener.py|env.example|\.env' . 2>/dev/null || true
echo "== listener outline/size =="
if [ -f listener.py ]; then
wc -l listener.py
ast-grep outline listener.py --view compact || true
echo "== relevant listener.py =="
sed -n '1,35p;180,225p;260,330p' listener.py
fi
echo "== load_env usages/definition =="
rg -n "def load_env|load_env\(|os\.getenv\(|XMCP_PROCESSED_MENTIONS_PATH|XMCP_MAX_PROCESSED_MENTIONS|XMCP_LAST_SEEN_PATH" . -g '!node_modules' -g '!build' -g '!dist' || true
echo "== env docs =="
if [ -f env.example ]; then
sed -n '1,160p' env.example
fiRepository: groupthinking/MyXstack
Length of output: 11637
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import ast
from pathlib import Path
listener = Path("listener.py").read_text()
tree = ast.parse(listener)
module_os_getenv_calls = []
load_env_body = []
main_load_env_calls = []
main_body = []
for node in ast.iter_child_nodes(tree):
if isinstance(node, ast.Assign):
if any(isinstance(t, ast.Name) and t.id == "LAST_SEEN_PATH" for t in node.targets
for t in node.targets if isinstance(t, ast.Name)) or \
any(isinstance(t, ast.Name) and t.id == "PROCESSED_MENTIONS_PATH" for t in node.targets
for t in node.targets if isinstance(t, ast.Name)) or \
any(isinstance(t, ast.Name) and t.id == "MAX_PROCESSED_MENTIONS" for t in node.targets
for t in node.targets if isinstance(t, ast.Name)):
for child in ast.walk(node):
if isinstance(child, ast.Call) and isinstance(child.func, ast.Attribute):
if child.func.attr == "getenv":
module_os_getenv_calls.append((node.lineno, child.func.value.id, child.func.attr))
if isinstance(node, ast.FunctionDef) and node.name == "load_env":
load_env_body.extend([child.lineno for child in ast.walk(node) if hasattr(child, "lineno")])
if isinstance(node, ast.FunctionDef) and node.name == "main":
main_load_env_calls.extend((start.lineno, child.lineno)
for start in node.body[:1] if isinstance(start, ast.Expr) and isinstance(start.value, ast.Call) and isinstance(start.value.func, ast.Name) and start.value.func.id == "load_env"
for child in [start])
main_body.extend([child.lineno for child in ast.walk(node) if hasattr(child, "lineno")])
print("module-level os.getenv calls:", module_os_getenv_calls)
print("load_env function line range approximately:", min(load_env_body) if load_env_body else None, max(load_env_body) if load_env_body else None)
print("main load_env call lines:", [lines[1] for lines in main_load_env_calls])
print("main lines:", min(main_body) if main_body else None, max(main_body) if main_body else None)
print("module-level os.getenv before load_env:", all(l < min(load_env_body) for _, _, l in module_os_getenv_calls))
print("main load_env lines:", sorted(main_load_env_calls))
PYRepository: groupthinking/MyXstack
Length of output: 539
.env-only ledger config is ignored.
PROCESSED_MENTIONS_PATH and MAX_PROCESSED_MENTIONS are bound at import time. load_env() runs later in main(), so .env values for these keys do not affect the ledger path or cap. Move load_env() before module-level config reads, or resolve these values lazily/inside main().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 15 - 23, Ensure load_env() runs before
PROCESSED_MENTIONS_PATH and MAX_PROCESSED_MENTIONS are evaluated, or resolve
both settings lazily after environment loading in main(). Preserve the existing
environment variable names and defaults so .env values control the ledger path
and cap.
| def load_processed_mentions() -> "set[str]": | ||
| """Load recently processed mention IDs, compacting the ledger on the way. | ||
|
|
||
| An unreadable ledger must not kill the listener thread — worst case a | ||
| few boundary mentions get a second reply, which is preferable to no | ||
| mentions being handled at all. | ||
| """ | ||
| try: | ||
| if not PROCESSED_MENTIONS_PATH.exists(): | ||
| return set() | ||
| lines = [ | ||
| line.strip() | ||
| for line in PROCESSED_MENTIONS_PATH.read_text(encoding="utf-8").splitlines() | ||
| if line.strip() | ||
| ] | ||
| except OSError as exc: | ||
| print( | ||
| f"WARNING: could not read {PROCESSED_MENTIONS_PATH}: {exc}; " | ||
| "starting with an empty processed-mentions set (duplicate replies possible)", | ||
| flush=True, | ||
| ) | ||
| return set() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fail-open on ledger read errors re-enables the exact duplicate-reply risk already flagged.
This still returns an empty set on any OSError, meaning a transient or permanent read failure resets duplicate suppression to zero and lets every boundary mention get replied to again. The docstring says this is intentional, but it's the same problem previously raised: retry or fail closed instead of quietly reopening the door to duplicate replies.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 45 - 66, Update load_processed_mentions so an
OSError while reading PROCESSED_MENTIONS_PATH does not return an empty
processed-mentions set; retry the ledger read or fail closed by preserving
duplicate suppression. Adjust the docstring and warning to reflect the chosen
behavior, while keeping normal loading and compaction unchanged.
| if len(lines) > MAX_PROCESSED_MENTIONS: | ||
| lines = lines[-MAX_PROCESSED_MENTIONS:] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
MAX_PROCESSED_MENTIONS=0 doesn't cap anything.
lines[-0:] is lines[0:] — the whole list. Setting the cap to 0 silently disables truncation instead of shrinking it to zero, and a negative value slices from the wrong end. Guard the boundary explicitly.
🔧 Proposed fix
- if len(lines) > MAX_PROCESSED_MENTIONS:
- lines = lines[-MAX_PROCESSED_MENTIONS:]
+ if MAX_PROCESSED_MENTIONS <= 0:
+ lines = []
+ elif len(lines) > MAX_PROCESSED_MENTIONS:
+ lines = lines[-MAX_PROCESSED_MENTIONS:]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if len(lines) > MAX_PROCESSED_MENTIONS: | |
| lines = lines[-MAX_PROCESSED_MENTIONS:] | |
| if MAX_PROCESSED_MENTIONS <= 0: | |
| lines = [] | |
| elif len(lines) > MAX_PROCESSED_MENTIONS: | |
| lines = lines[-MAX_PROCESSED_MENTIONS:] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 67 - 68, Update the truncation logic near
MAX_PROCESSED_MENTIONS to handle zero and negative limits explicitly: a
non-positive cap must produce an empty lines list, while positive caps retain
only the last MAX_PROCESSED_MENTIONS entries when the list exceeds the limit.
| processed_mentions.add(mention_id) | ||
| try: | ||
| save_processed_mention(mention_id) | ||
| except OSError as exc: | ||
| # The reply already went out — a persistence failure only | ||
| # risks a duplicate after restart, so log it as such | ||
| # rather than as a reply failure. | ||
| print( | ||
| f"WARNING: could not persist processed mention {mention.id}: {exc}", | ||
| flush=True, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Persistence failure here still produces a duplicate reply after restart.
When save_processed_mention() raises OSError, the code logs a warning and continues; start_time still advances past this mention right after. On restart, load_last_seen() resumes exactly at this mention, load_processed_mentions() does not contain its ID (the write failed), and it gets replied to a second time. This reproduces the previously flagged risk of treating a persistence failure as harmless when the reply already went out non-idempotently.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@listener.py` around lines 265 - 275, Update the exception path around
save_processed_mention in the mention-processing flow so an OSError does not
allow start_time to advance past an unpersisted mention; stop or otherwise retry
processing before advancing the checkpoint, while preserving the existing
warning context and normal success behavior.
Re-cut from current
main(per the review decision to re-cut rather than hand-resolve six months of branch drift) and re-applies the original intent:listener.py: persist replied-to mention IDs toXMCP_PROCESSED_MENTIONS_PATHand skip them on re-fetch — the inclusivestart_timewatermark re-returns the boundary mention on every restart, causing duplicate replies. The watermark still advances on skips, the ledger is compacted toXMCP_MAX_PROCESSED_MENTIONSentries on load, an unreadable ledger degrades to an empty set instead of killing the listener thread, and persistence failures are logged distinctly from reply failures.agents/team/general.py: replace the"Thinking..."placeholder published when Grok returns nothing with an apology fallback (where the original fallback-message change now lives after the agent-team refactor).env.example: document the new state variables.The original
TIMELINE_API_URLport change (8080 → 8000) is dropped per review feedback — the rest of the repo defaults to 8080.